Use the block-mysql driver by morgo · Pull Request #24 · block/polt

morgo · 2026-09-06T19:44:51Z

github.com/block/mysql is Block's fork of go-sql-driver/mysql. It is moving to
its own module path in block/mysql#3, because a replace directive is not
inherited across module boundaries — a consumer building against a library that
depends on the fork would silently link upstream instead.

polt reaches the driver through spirit's dbconn, and spirit is moving with it
(block/spirit#1219), so polt follows. Keeping one driver in the binary also keeps
one *mysql.MySQLError type: with both linked, errors.As across the boundary
returns false silently.

The change

Two mechanical substitutions across 12 files:

import path github.com/go-sql-driver/mysql → github.com/block/mysql
sql.Open("mysql", ...) → sql.Open("block-mysql", ...)

There is only one import form in the tree (no aliases), and only one non-test
file: pkg/test/test.go.

No API had to change. pkg/test's SetupDB(cfg *mysql.Config, ...) reads
User/Passwd/Addr/DBName off the config to build a DSN string rather than
handing the struct to spirit, so no driver type crosses a module boundary. (strata
does hand a *mysql.Config to applier.Target, which is why strata does not
compile until spirit moves; polt has no such site.) There are no MySQLError
assertions anywhere in polt.

go-sql-driver leaves go.mod entirely — not even as an indirect dependency —
so polt is single-driver after this.

Pins

Both are on merged code — nothing outstanding:

Module	Pin	Merged as
`github.com/block/mysql`	`v0.0.0-20260906201522-a3178f8dca69`	block/mysql#3, head of `master`
`github.com/block/spirit`	`v0.17.1-0.20260906214441-dc3d4c9f4c3b`	block/spirit#1219, ancestor of `main`

Verification

go build ./..., go vet ./... and gofmt clean, with no residual references to
the old import path or driver name. README's Dependencies section now names the fork. Full suite against MySQL 8.0.44 — all 8
packages pass (archive, audit, boot, parquet, query, runner, stage,
upload).

block/mysql is Block's fork of go-sql-driver/mysql, moving to its own module path (block/mysql#3) because a `replace` directive is not inherited across module boundaries. polt reaches the driver through spirit's dbconn, and spirit is moving with it (block/spirit#1219), so polt follows to keep one driver — and one set of driver types — in the binary. Mechanical: the import path, and `sql.Open("mysql", ...)` -> `"block-mysql"`. Nothing crosses a type boundary here; pkg/test's SetupDB reads fields off *mysql.Config to build a DSN string rather than handing the struct to spirit, so no API had to change. go-sql-driver leaves go.mod entirely, including as an indirect dependency. The block/mysql and block/spirit pins point at unmerged PR branches and must be re-pointed at merged commits before this merges. Verified: build, vet and gofmt clean; all 8 packages pass against MySQL 8.0.44.

Copilot

🟡 Changes recommended

The go.mod pins github.com/block/mysql and github.com/block/spirit to pseudo-versions that appear to track unmerged PR branches and should be repointed to merged commits/tags before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR migrates polt from github.com/go-sql-driver/mysql to Block’s fork (github.com/block/mysql) and updates MySQL driver usage to ensure the intended driver is linked across module boundaries (including consistent MySQLError typing).

Changes:

Replaces imports of github.com/go-sql-driver/mysql with github.com/block/mysql across tests and pkg/test.
Updates sql.Open("mysql", ...) calls to sql.Open("block-mysql", ...) across the test suite.
Updates module dependencies to include github.com/block/mysql and newer github.com/block/spirit (plus associated go.sum churn).

File summaries

File	Description
pkg/test/test.go	Switches driver import and updates `RunSQL` to open the `block-mysql` driver.
pkg/stage/stager_test.go	Updates MySQL driver import and `sql.Open` driver name.
pkg/runner/stage_runner_test.go	Updates MySQL driver import and `sql.Open` driver name.
pkg/runner/runner_test.go	Updates `sql.Open` driver name to `block-mysql`.
pkg/runner/archive_runner_test.go	Updates MySQL driver import and `sql.Open` driver name.
pkg/query/validator_test.go	Updates MySQL driver import and `sql.Open` driver name.
pkg/query/expression_folder_test.go	Updates MySQL driver import and `sql.Open` driver name (including DSN query params case).
pkg/query/explain_test.go	Updates `sql.Open` driver name to `block-mysql`.
pkg/parquet/write_buffer_test.go	Updates `sql.Open` driver name to `block-mysql`.
pkg/parquet/arrow_schema_test.go	Updates `sql.Open` driver name to `block-mysql`.
pkg/boot/stage_booter_test.go	Updates MySQL driver import and `sql.Open` driver name (incl. replica/open-invalid-DSN cases).
pkg/boot/booter_test.go	Updates MySQL driver import for DSN parsing compatibility.
pkg/boot/archive_booter_test.go	Updates MySQL driver import and `sql.Open` driver name.
pkg/audit/db_test.go	Updates `sql.Open` driver name to `block-mysql`.
pkg/archive/table_test.go	Updates MySQL driver import and `sql.Open` driver name.
pkg/archive/file_test.go	Updates MySQL driver import and `sql.Open` driver name.
pkg/archive/buffer_stager_test.go	Updates MySQL driver import and `sql.Open` driver name.
go.mod	Replaces go-sql-driver with block/mysql; updates spirit pin; also bumps testify.
go.sum	Updates checksums to reflect dependency migration and version changes.

Review details

Suppressed comments (1)

go.mod:20

go.mod pins github.com/block/spirit to a pseudo-version that appears to reference an unmerged PR branch; this should be updated to a merged commit/tag before merging to avoid depending on a mutable branch reference.

	github.com/block/spirit v0.17.1-0.20260906193437-6f267e49110f

Files reviewed: 18/19 changed files
Comments generated: 2
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

 	github.com/aws/aws-sdk-go-v2/config v1.29.1
 	github.com/aws/aws-sdk-go-v2/service/s3 v1.73.2
-	github.com/go-sql-driver/mysql v1.10.0
+	github.com/block/mysql v0.0.0-20260906185715-6346a1ec80d2


 	github.com/siddontang/loggers v1.0.3
 	github.com/sirupsen/logrus v1.9.3
-	github.com/stretchr/testify v1.12.0
+	github.com/stretchr/testify v1.12.1
 	golang.org/x/sync v0.22.0


aparajon · 2026-09-06T20:59:50Z

🤖 Adversarial correctness review — 784441b1 (+73/-90, 19 files)

Unlike the sibling change in block/vitess, this one actually reaches the single-driver state the body claims, and that is worth establishing first because it is what makes a 31-site mechanical rename safe. go list -m all has no go-sql-driver entry at all — not direct, not indirect — and I probed the built test binary rather than inferring from go.mod:

PROBE drivers=[block-mysql]
PROBE open_mysql_err=sql: unknown driver "mysql" (forgotten import?)

So a site you missed cannot silently bind to upstream; it fails at the sql.Open. I checked that this is real rather than theoretical by mutating one back — pkg/audit/db_test.go to sql.Open("mysql", …), image rebuilt — and all three tests in the package fail with exactly that error. Every go.mod line in the diff is MVS doing its job rather than hand-editing, and every claim in the body about polt's own surface checks out: no MySQLError, no errors.As, no Register* call anywhere in the repo, and no driver type crossing into spirit.

What I would change before merge is a pin and a README line.

#	Sev	Where	What
1	low-med	`go.mod:20`, PR body	The spirit pin is still on the open block/spirit#1219 branch, and the body's "Before merge" list is stale — it names both pins at their pre-`784441b` SHAs and gates on leaving draft, which has already happened
2	low	`README.md:58`	The Dependencies section still tells readers polt relies on go-sql-driver, which after this PR is not in the module graph at all

1 — one of the two pins is discharged and the body no longer says which (low-med)

784441b did the block/mysql half properly: a3178f8d is the squash-merge of block/mysql#3 and the current head of that repo's master, so that pin is on merged code. The spirit half can't be discharged yet — v0.17.1-0.20260906201921-2d412cd16671 is the head of the still-open block/spirit#1219, on branch driver/block-mysql, 2 ahead and 0 behind spirit main.

The body hasn't caught up. Its "Before merge" section still lists both pins, at their pre-784441b SHAs:

github.com/block/mysql v0.0.0-20260906185715-6346a1ec80d2 → Make this a real fork: module path github.com/block/mysql, driver name block-mysql mysql#3

github.com/block/spirit v0.17.1-0.20260906193437-6f267e49110f → Use the block-mysql driver spirit#1219

Both must be re-pointed at merged commits before this leaves draft.

Neither SHA is in go.mod any more, one of the two items is already done, and the gate it states has been passed — the PR is out of draft and up for review. A reviewer reading top-down can't tell what's actually outstanding, which is the whole job of that section.

I went looking for what breaks if this merges ahead of #1219, and the obvious fear turns out not to be the risk, so it's worth writing down which one is:

Not a build break. proxy.golang.org already has the pseudo-version cached (.info returns 200) and proxy entries are immutable, so resolution is permanent. delete_branch_on_merge is false on block/spirit, so driver/block-mysql survives the merge regardless.
The cost is silent divergence. spirit squash-merges — I checked the last ten merged PRs and merge_commit_sha != head.sha in every one — so once #1219 lands, the same change is on main under a different SHA and polt's main is pinned to a commit that is not an ancestor of spirit main. polt then stops tracking spirit from a point nobody can git log to, and every later spirit commit is invisible until someone re-pins by hand.
And it leaks through MVS. v0.17.1-0.2026… sorts above v0.17.0 and below a future v0.17.1 tag, so any module that composes polt with an older spirit silently gets the PR-branch build.

Re-pin after #1219 merges, and rewrite the list to name only the outstanding pin.

2 — the Dependencies section still points at the old driver (low)

## Dependencies
* Relies on [mysql-client-driver](https://github.com/go-sql-driver/mysql) for connecting to MySQL databases.
* Relies on [Spirit](https://github.com/block/spirit) for chunking the data ...

This is the only remaining reference to go-sql-driver anywhere in the repo — one grep hit, no others in Go files, workflows, compose or scripts. It is also the one place a reader is now told something false: after this PR upstream is absent from go.mod, go.sum and go list -m all. Pointing it at github.com/block/mysql and saying in a clause that it's Block's fork of go-sql-driver is the whole fix, and it sits two lines above the Spirit bullet that the same PR effectively updates.

Same category, not its own row: "block-mysql" is now a literal in 31 places across 16 test files, and 26 of them are the identical pair —

db, err := sql.Open("block-mysql", test.DSN())
require.NoError(t, err)

Every one of those files already imports pkg/test, which already owns DSN(). A test.OpenDB(t) *sql.DB there would put the driver name and the DSN in the one place that should hold both, and would make the next driver move a one-line change rather than a 31-line one. Cheap to do while all the sites are open anyway; a package-level const driverName is the smaller version if the helper feels like scope creep.

Verified — the local runs, the mutation, and four attacks that dissolved

Local: go build ./..., go vet ./... and gofmt -l all clean. go mod tidy is a no-op on this tree, so the module files are exactly what the toolchain produces. The suite ran the way CI runs it (compose up mysql test, MySQL 8.0.34): all 8 packages green at HEAD from a cold database and a fresh image, and green again warm. I also ran the same cold suite at origin/main for a baseline — also green.

Mutation. pkg/audit/db_test.go sql.Open("block-mysql", …) → "mysql", image rebuilt: killed, all three tests failing with sql: unknown driver "mysql" (forgotten import?). So the rename is self-verifying here — this is the property block/vitess has and block/mysql itself does not, and it is what makes reviewing 31 call sites by grep rather than by eye defensible.

⚠️ One flake worth knowing about, not caused by this PR: on the very first cold run I saw TestStageBooter_Setup fail at stage_booter_test.go:42 (TableExists("polt", "runs") false), and it did not reproduce in three subsequent cold and warm runs, at HEAD or at base. The mechanism is pre-existing and independent of the driver: test.TableExists does _ = db.QueryRowContext(...).Scan(&count) and discards the error, so any transient query failure under concurrent package load reads as "table absent" rather than as an error. Worth a require.NoError there sometime — a swallowed error in a boolean helper is exactly the shape that turns a resource problem into a mystery assertion failure.

Attack that dissolved: the go.mod churn beyond the driver swap. A testify bump and six golang.org/x/* bumps inside a driver PR are worth questioning; all of it is MVS. spirit@2d412cd1 requires testify v1.12.1 (raising polt's direct v1.12.0) and golang.org/x/text v0.41.0; testify v1.12.1 in turn swapped gopkg.in/yaml.v3 for go.yaml.in/yaml/v3 v3.0.5, which is why yaml.v3 leaves and kr/pretty and rogpeppe/go-internal — its gopkg.in/check.v1 test chain — go with it. That accounts for the whole 55-line go.sum movement, including the net shrink.

Attack that dissolved: an exported signature whose type identity changed. pkg/test.SetupDB(cfg *mysql.Config, …) is exported from a public module, and *mysql.Config is a different type after the module path change — a source break for any external importer. Public code search for github.com/block/polt/pkg returns 5 hits, all inside block/polt itself, so there is nothing to break. (Incidentally SetupDB rebuilds the DSN with fmt.Sprintf("%s:%s@tcp(%s)/%s", …) where cfg.FormatDSN() would do it without dropping params or mangling a password containing @ or / — pre-existing and test-only, but it is the reason this file needs the driver import at all.)

Attack that dissolved: polt's production driver isn't actually set by this diff. Only one non-test file changed, and no production path calls sql.Open — everything goes through spirit's dbconn.New (pkg/runner/db_helper.go:41). So polt's real driver is spirit's choice, and dropping go-sql-driver from go.mod would be a runtime failure if the pinned spirit still opened "mysql". It doesn't: all 14 non-test sql.Open sites in spirit@2d412cd1 say "block-mysql" and none say "mysql", and its four remaining go-sql-driver strings are all comments (dbconn/conn.go:324,342, copier/buffered.go:195, migration/check/version.go:19). The pin and the driver drop are consistent.

Leak check: clean. Nothing internal in the diff. The body names strata and its applier.Target call, but applier.Target with its Config *mysql.Config field is a public symbol in block/spirit/pkg/applier, and strata is already named in public Block source, so the only thing stated is that a Block consumer calls a public spirit API — no new disclosure.

The probe file was moved out of the tree and the mutation restored from backup; the worktree is clean at 784441b1.

This review was generated by Claude Code (claude-opus-5).

aparajon

🤖 Approving. Two low-severity items in the review above: the spirit pin needs re-pointing once block/spirit#1219 merges, and README.md:58 still names go-sql-driver. Neither is a correctness problem — the substitution is complete and self-verifying (mutation killed, full suite green cold and warm).

This stamp was left by Claude Code (claude-opus-5).

… fork block/spirit#1219 squash-merged as dc3d4c9f, so the pin moves off the PR branch and onto a commit that is an ancestor of spirit main. The Dependencies section still named go-sql-driver, which after this change is not in polt's module graph at all.

Copilot AI lite review requested due to automatic review settings September 6, 2026 19:44

Copilot started reviewing on behalf of morgo September 6, 2026 19:45 View session

Copilot AI reviewed Sep 6, 2026

View reviewed changes

Pin block/mysql at merged master (block/mysql#3)

784441b

morgo marked this pull request as ready for review September 6, 2026 20:26

aparajon approved these changes Sep 6, 2026

View reviewed changes

aparajon mentioned this pull request Sep 6, 2026

Use the block-mysql driver block/spirit#1219

Merged

morgo enabled auto-merge (squash) September 6, 2026 21:47

morgo merged commit 9c18be0 into main Sep 6, 2026
13 of 14 checks passed

Navigation Menu

Uh oh!

Use the block-mysql driver - #24

Use the block-mysql driver#24
morgo merged 3 commits into
mainfrom
driver/block-mysql

morgo commented Sep 6, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

aparajon commented Sep 6, 2026

Uh oh!

aparajon left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

morgo commented Sep 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The change

Pins

Verification

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (1)

Uh oh!

aparajon commented Sep 6, 2026

1 — one of the two pins is discharged and the body no longer says which (low-med)

2 — the Dependencies section still points at the old driver (low)

Uh oh!

aparajon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

morgo commented Sep 6, 2026 •

edited

Loading